Skip to content

Expose Wasi-TLS handshake error#10429

Merged
alexcrichton merged 3 commits intobytecodealliance:mainfrom
badeend:wasi-tls-errors
Mar 21, 2025
Merged

Expose Wasi-TLS handshake error#10429
alexcrichton merged 3 commits intobytecodealliance:mainfrom
badeend:wasi-tls-errors

Conversation

@badeend
Copy link
Member

@badeend badeend commented Mar 20, 2025

Implements WebAssembly/wasi-tls#10

And added a test case to verify the error information is indeed reaching the guest.

PR is still draft, because above issue needs to be merged first. But feel free to review the design & implementation already.

CC @jsturtevant

@badeend badeend requested a review from a team as a code owner March 20, 2025 14:37
@badeend badeend requested review from alexcrichton and removed request for a team March 20, 2025 14:37
@badeend badeend changed the title Expose Wasi-TLS handshake error Draft: Expose Wasi-TLS handshake error Mar 20, 2025
@badeend badeend marked this pull request as draft March 20, 2025 14:40
@github-actions github-actions bot added the wasi Issues pertaining to WASI label Mar 20, 2025
let this = &mut self.table.get_mut(&this)?.0;
match this {
StreamState::Pending(_) => return Ok(None),
StreamState::Closed => return Ok(Some(Err(()))),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we be return some information about the stream being closed with this error?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok(s) => s,
Err(TlsError::Trap(e)) => return Err(e),
Err(TlsError::Io(e)) => {
let error = self.table.push(e)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need logic to clean up these table entries?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean the table entries for the newly created wasi:io/error resources? The ownership of these resource is transferred into the guest one line below it. It's then the responsibility of the guest to drop it properly like any other resource. When the guest drops that resource, wasmtime removes it from the table in https://github.com/bytecodealliance/wasmtime/blob/main/crates/wasi-io/src/impls.rs#L114-L116

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the explanation

@badeend badeend marked this pull request as ready for review March 20, 2025 19:54
@badeend badeend requested a review from a team as a code owner March 20, 2025 19:54
@badeend badeend changed the title Draft: Expose Wasi-TLS handshake error Expose Wasi-TLS handshake error Mar 20, 2025
@badeend
Copy link
Member Author

badeend commented Mar 20, 2025

I've updated ci/vendor-wit.sh to match the new WITs. This is ready for final review now.

Copy link
Contributor

@jsturtevant jsturtevant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thanks for the updates

@alexcrichton alexcrichton added this pull request to the merge queue Mar 21, 2025
Merged via the queue into bytecodealliance:main with commit 0ca6d4f Mar 21, 2025
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wasi Issues pertaining to WASI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments